-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Forward #971
base: master
Are you sure you want to change the base?
Forward #971
Conversation
…tattribute__. This makes a much nicer access pattern for self._instance / self._symbol, but performance should be checked to compare the two. This also fixes the issubclass check, which had the operands in the wrong order.
…tring pre-and-post startJVM.
The major change being that we always use __getattr__ instead of __getattribute__
Codecov Report
@@ Coverage Diff @@
## master #971 +/- ##
==========================================
- Coverage 87.54% 86.04% -1.50%
==========================================
Files 114 116 +2
Lines 10076 10356 +280
Branches 4063 4106 +43
==========================================
+ Hits 8821 8911 +90
- Misses 642 833 +191
+ Partials 613 612 -1
Continue to review full report at Codecov.
|
This pull request introduces 2 alerts when merging 359b5e8 into 6e9a5b1 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 0b61aad into 661c541 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>>> import jpype.jroot.java
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 674, in _load_unlocked
File "<frozen importlib._bootstrap>", line 571, in module_from_spec
File "jpype/_jforward.py", line 243, in create_module
mod = self._load_module(spec.name)
File "jpype/_jforward.py", line 256, in _load_module
return JForward(java_name)
File "jpype/jpype/_jforward.py", line 169, in __new__
return _lookup(symbol)
File "jpype/jpype/_jforward.py", line 19, in _lookup
if _jpype.isPackage(symbol):
jpype._core.JVMNotRunning: Java Virtual Machine is not running
|
||
def __new__(cls, symbol: typing.Optional[str]): | ||
if not _jpype.isStarted(): | ||
return _lookup(symbol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can conclude that _lookup
needn't have the JVM running, right?
def _lookup(symbol: typing.Optional[str]): | ||
if symbol is None: | ||
return None | ||
if _jpype.isPackage(symbol): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This errors if the JVM isn't running.
Clearly we stalled on this one (totally understandable). I was hoping to at least review what is here already, but it seems that the fundamentals aren't yet working (e.g. |
Some really valuable insight in #1176 (comment). I plan to respond to the specific JForward points here, but will focus on that MR for now. |
This is the prototype code for JForword which allows resources to be created prior to starting the JVM. The internal methods are all working, but it requires clean up, hooking into the main functionality so that stubs are created automatically, and testing.
This one will be pretty difficult to test as it requires playing with behaviors like weak referencing and JVM starting. The easy tests are to manually create a forward after the JVM is started.